Skip to content

Conversation

@qnixsynapse
Copy link
Contributor

@qnixsynapse qnixsynapse commented Aug 19, 2025

Describe Your Changes

Re enable reasoning_content in the llamacpp provider extension for a single standard API for rendering reasoning content

Fixes Issues

  • Closes #
  • Closes #

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Re-enable reasoning_content in llamacpp extension, update reasoning extraction logic, and add tests for reasoning processing.

  • Behavior:
    • Re-enable reasoning_content in AIEngine.ts and index.ts by removing --reasoning-format none.
    • Update extractReasoningFromMessage() in reasoning.ts to prefer reasoning_content over reasoning.
  • Tests:
    • Add reasoning.test.ts to test reasoning extraction and processing.
    • Cover scenarios for reasoning_content, reasoning, and mixed content.
  • Misc:
    • Add getReasoning() helper function in reasoning.ts for extracting reasoning content.

This description was created by Ellipsis for 52a8fd0. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to e0e58d6 in 57 seconds. Click for details.
  • Reviewed 12 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. extensions/llamacpp-extension/src/index.ts:1212
  • Draft comment:
    Removed the '--reasoning-format none' flag to re-enable reasoning content. Ensure the backend now defaults to producing reasoning output as expected and update docs/tests if necessary.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to ensure that the backend defaults to producing reasoning output and to update docs/tests if necessary. This falls under the rule of not asking the author to ensure behavior or make sure changes are tested. Therefore, this comment should be removed.

Workflow ID: wflow_OAFP2HHpaQY7tha9

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 19, 2025

Barecheck - Code coverage report

Total: 37.81%

Your code coverage diff: 0.22% ▴

✅ All code changes are covered

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 2782b65 in 1 minute and 20 seconds. Click for details.
  • Reviewed 431 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. core/src/browser/extensions/engines/AIEngine.ts:10
  • Draft comment:
    Clarify that 'reasoning_content' takes precedence over the legacy 'reasoning'. Adding a note here can improve developer understanding.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is suggesting adding a note for clarification, which is a request for purely informative content. This violates the rule against making purely informative comments.
2. web-app/src/utils/reasoning.ts:43
  • Draft comment:
    Consider documenting the behavior when a chunk contains both reasoning and content tokens. The current logic always prioritizes reasoning if present, which may be intentional but could use explicit clarification.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. web-app/src/utils/reasoning.ts:36
  • Draft comment:
    For consistency, consider renaming the internal state variable 'isReasoningActive' to something like 'isReasoningInProgress' to match the getter method name.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_9hnq3DrnGLP5lmqx

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@dinhlongviolin1 dinhlongviolin1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed ff63e33 in 1 minute and 50 seconds. Click for details.
  • Reviewed 34 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/utils/reasoning.ts:20
  • Draft comment:
    Refactor: Using getReasoning in extractReasoningFromMessage cleans up the logic. This improves maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. web-app/src/utils/reasoning.ts:30
  • Draft comment:
    Consistently applying getReasoning in extractReasoningFromChunk ensures uniform handling of reasoning content across different contexts.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_7FJ6tFhJfQ8dHmC1

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 52a8fd0 in 1 minute and 22 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/utils/reasoning.ts:10
  • Draft comment:
    Using the nullish coalescing operator (??) here is intentional to prefer a provided empty string over falling back to 'reasoning'. Confirm that empty strings in 'reasoning_content' should be preserved rather than triggering the fallback.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_1AKB9p2GzgWg9qJ9

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@qnixsynapse qnixsynapse linked an issue Aug 19, 2025 that may be closed by this pull request
3 tasks
@qnixsynapse qnixsynapse merged commit 906b870 into dev Aug 20, 2025
16 checks passed
@qnixsynapse qnixsynapse deleted the chore/reasoning_content branch August 20, 2025 07:36
@github-project-automation github-project-automation bot moved this to QA in Jan Aug 20, 2025
@github-actions github-actions bot added this to the v0.6.9 milestone Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

bug: llama.cpp with local gpt-oss not usable

4 participants